INTPYTHON-355 Add transaction support #313
Conversation
aclark4life
changed the title
django_mongodb_backend/features.py
Outdated
| @@ -97,6 +95,22 @@ class DatabaseFeatures(BaseDatabaseFeatures): | |||
| "expressions.tests.ExpressionOperatorTests.test_lefthand_transformed_field_bitwise_or", | |||
| } | |||
|
|
|||
| @cached_property | |||
| def supports_transactions(self): | |||
There was a problem hiding this comment.
What's the purpose of this property? Is it just for the test suite or do apps rely on it?
There was a problem hiding this comment.
Not just for testing.
In this case, rather than declare False or True for supports_transactions we dynamically set True or False based on what type of MongoDB we are connected to.
Then DatabaseFeatures is passed to DatabaseWrapper IIRC and the runtime behavior of Django changes accordingly.
Based on this: https://github.com/django/django/blob/main/django/db/backends/base/features.py#L419-L431
There was a problem hiding this comment.
It depends on what we want to implement. If we want to dynamically enable transactions based on whether or not the database supports it, we could use it internally. Otherwise, we'll have to introduce a separate setting the user sets to indicate whether or not they want to use transactions, and raise an error if it's not supported.
It's also used by Django in a couple places. For example, to determine whether or not to use transactions to speed up tests.
There was a problem hiding this comment.
The issue is that there are certain storage engines where a replica set or sharded cluster does not actually support transactions so this check isn't 100% accurate. That said, those should be rare so I'm fine with the current "hello" based approach. We can always revisit later if it causes an issue.
There was a problem hiding this comment.
Fixed in c8f8881 (assuming wired tiger storage engine is correct or we could also check to make sure it is not mmapv1)
django_mongodb_backend/features.py
Outdated
| @@ -97,6 +95,22 @@ class DatabaseFeatures(BaseDatabaseFeatures): | |||
| "expressions.tests.ExpressionOperatorTests.test_lefthand_transformed_field_bitwise_or", | |||
| } | |||
|
|
|||
| @cached_property | |||
| def supports_transactions(self): | |||
There was a problem hiding this comment.
The issue is that there are certain storage engines where a replica set or sharded cluster does not actually support transactions so this check isn't 100% accurate. That said, those should be rare so I'm fine with the current "hello" based approach. We can always revisit later if it causes an issue.
aclark4life
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
timgraham
force-pushed
the
INTPYTHON-355
branch
2 times, most recently
from
e68f8c9 to
4a8209a
Compare
| name: Django Test Suite | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout django-mongodb-backend | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| persist-credentials: false | ||
| - name: install django-mongodb-backend | ||
| run: | | ||
| pip3 install --upgrade pip | ||
| pip3 install -e . | ||
| - name: Checkout Django | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: 'mongodb-forks/django' | ||
| ref: 'mongodb-5.2.x' | ||
| path: 'django_repo' | ||
| persist-credentials: false | ||
| - name: Install system packages for Django's Python test dependencies | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install libmemcached-dev | ||
| - name: Install Django and its Python test dependencies | ||
| run: | | ||
| cd django_repo/tests/ | ||
| pip3 install -e .. | ||
| pip3 install -r requirements/py3.txt | ||
| - name: Copy the test settings file | ||
| run: cp .github/workflows/mongodb_settings.py django_repo/tests/ | ||
| - name: Copy the test runner file | ||
| run: cp .github/workflows/runtests.py django_repo/tests/runtests_.py | ||
| - name: Start MongoDB | ||
| uses: supercharge/[email protected] | ||
| with: | ||
| mongodb-version: 6.0 | ||
| mongodb-replica-set: test-rs | ||
| - name: Run tests | ||
| run: python3 django_repo/tests/runtests.py --settings mongodb_settings -v 2 |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium test
timgraham
force-pushed
the
INTPYTHON-355
branch
3 times, most recently
from
8205175 to
1c784eb
Compare
Referring to tests defined in |
Transactions are tested in Atlas tests.
timgraham
force-pushed
the
INTPYTHON-355
branch
2 times, most recently
from
763a039 to
f007803
Compare
timgraham
force-pushed
the
INTPYTHON-355
branch
2 times, most recently
from
d02ce65 to
4e48cec
Compare
I didn't try running the test suite on a sharded cluster. Maybe this could be done at a later date.